Move the indication of destination message type into Codec #599
Conversation
jhump
changed the title
|
Actually...should we do the same on the marshaling side? |
…om Codec could obscure it
…d in first commit)
We don't actually include the type anywhere in marshal errors today. Instead we just return an internal error with "marshal message" as the error context. But while searching, I did totally miss a spot that needed to be updated 🤦. I had updated only one spot where we unmarshal messages -- in the GET part of the Connect protocol. The other part (that handles all other protocols, unmarshalling from the body) is now also fixed. For consistency, the code now just adds "unmarshal message" as error context, to decorate whatever the codec returns. It is up to the codec to include any other details (such as the target or source type). Do you think I should add the type to the default codecs' marshal methods? (I assumed not and that your suggestion was just to be sure we were consistent for both halves of the codec.) |
Nope! This LGTM, let's merge it. |
Following connectrpc/connect-go#599, make the errors returned by this package's codecs include the protobuf type name.
Following connectrpc/connect-go#599, make the errors returned by this package's codecs include the protobuf type name.
This is so that a custom
Codeccan choose what details to reveal to clients, in cases where emitting the destination message type is considered to leak too much information.See #584 for more context.